Skip to content

[Run Service] Fix CreateRun response fields and fill missing default …#7058

Merged
pingsutw merged 1 commit intoflyteorg:v2from
WangWang0226:Ensure-CreateRun-works-expected
Mar 27, 2026
Merged

[Run Service] Fix CreateRun response fields and fill missing default …#7058
pingsutw merged 1 commit intoflyteorg:v2from
WangWang0226:Ensure-CreateRun-works-expected

Conversation

@WangWang0226
Copy link
Copy Markdown

@WangWang0226 WangWang0226 commented Mar 19, 2026

Tracking issue

#6972

Why are the changes needed?

This PR addresses gaps in the CreateRun workflow to ensure consistent behavior and a complete API response.

Specifically:

  1. Missing default input handling
    When users do not provide certain inputs, CreateRun should populate them using the task’s default_inputs, without overriding user-provided values.

  2. Incomplete response fields
    The CreateRun response previously lacked essential fields required by downstream consumers, such as metadata and execution status.


What changes were proposed in this pull request?

1. Add fillDefaultInputs to CreateRun

File: runs/service/run_service.go

  • Introduced s.fillDefaultInputs(ctx, req.Msg) before persisting inputs.

  • Logic:

    • Build a set of existing input names from the request
    • Iterate through taskSpec.default_inputs
    • Fill missing inputs only if a default value exists
    • Preserve user-provided values (no overwrite)
    • Use proto.Clone to avoid shared pointer side effects

2. Add getCreateRunTaskSpec

File: runs/service/run_service.go

Supports both task specification sources:

  • CreateRunRequest_TaskSpec: use inline spec directly
  • CreateRunRequest_TaskId: fetch from TaskRepo.GetTask and proto.Unmarshal

This ensures consistent handling regardless of how the task is provided.


3. Introduce a dedicated response builder for CreateRun

File: runs/service/run_service.go

  • CreateRun now returns:

    • buildCreateRunResponse(run)
  • The response includes:

    • run.action.metadata
    • run.action.status.phase
    • run.action.status.start_time
    • run.action.status.attempts = 1
    • run.action.status.cache_status = CACHE_DISABLED
  • Fields intentionally excluded at creation time:

    • end_time
    • duration_ms

Design decision: Not passing actionID into buildCreateRunResponse

Instead of passing actionID, it is reconstructed from the persisted run.

Rationale:

  1. Single source of truth
    Ensures the response is fully derived from the persisted state, avoiding inconsistencies.

  2. Better testability
    The function depends only on run, making it easier to test in isolation.

  3. Reduced coupling
    Avoids passing intermediate variables across layers solely for response construction.

Note: Passing actionID is technically valid, but this approach was chosen for cleaner design.


Why not modify convertRunToProto?

convertRunToProto is shared by ListRuns and WatchRuns.

To avoid unintended side effects or breaking existing contracts, this PR isolates CreateRun response logic in a separate builder (buildCreateRunResponse).


How was this patch tested?

Unit Tests (Added)

File: runs/service/run_service_test.go

  • TestFillDefaultInputs

    • Verifies user inputs are not overridden
    • Verifies missing inputs are populated from defaults
  • TestCreateRunResponseIncludesMetadataAndStatus

    • Verifies presence of metadata and status

    • Verifies:

      • phase
      • startTime
      • attempts
      • cacheStatus

Test Results

GOCACHE=/tmp/gocache go test ./runs/service
# ok

Local Integration Test Steps

  1. Start dev environment
make sandbox-run FLYTE_DEV=True
make -C manager run
  1. Call CreateRun
ENDPOINT=http://localhost:30080 bash runs/test/scripts/create_run.sh
  1. Validate response fields
jq '.run.action' /tmp/create_run_resp.json
jq -e '.run.action.metadata != null' /tmp/create_run_resp.json
jq -e '.run.action.status != null' /tmp/create_run_resp.json
jq -e '.run.action.status.phase == "ACTION_PHASE_QUEUED"' /tmp/create_run_resp.json
jq -e '.run.action.status.startTime != null' /tmp/create_run_resp.json
jq -e '.run.action.status.attempts == 1' /tmp/create_run_resp.json

Known Observations

  • When cacheStatus is set to CACHE_DISABLED (enum=0), it may be omitted in proto3 JSON output.
  • This is expected serialization behavior and does not indicate a backend issue.

Labels

  • added
  • changed

Setup process

See local integration steps above.


Screenshots

N/A (backend/API change)


Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

N/A


Docs link

N/A

  • main
    • Flyte 2 WIP #6583
      • [Run Service] Fix CreateRun response fields and fill missing default … 👈

@github-actions github-actions bot mentioned this pull request Mar 19, 2026
3 tasks
@pingsutw pingsutw added this to the V2 GA milestone Mar 21, 2026
@pingsutw
Copy link
Copy Markdown
Member

@WangWang0226 could you rebase the PR

Signed-off-by: WangWang0226 <eeha8834@gmail.com>
@WangWang0226 WangWang0226 force-pushed the Ensure-CreateRun-works-expected branch from 639b235 to e4fe24f Compare March 26, 2026 03:01
@WangWang0226
Copy link
Copy Markdown
Author

Hi @pingsutw, I rebased the branch and resolved the conflicts. This PR now only adds tests for fillDefaultInputs and the CreateRun response contract.

Copy link
Copy Markdown
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@pingsutw pingsutw merged commit dfb45ae into flyteorg:v2 Mar 27, 2026
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants